code review的几条规则
规则1
每个 PR 审查至少需要 2 个同组开发者的批准,管理者的审批不统计。
首先要注意的是,由于我所在的是一个 3 人团队,这是最理想的。所有的 修改 3 个开发者都 100% 知情。如果团队规模更大,情况可能会有所不同。你所追求的是邓布利多和死亡圣器(Dumbledore Horcruxes)的情况,如果你死了,至少有 2 - 3 人知道死亡圣器的事。我们的团队现在有 5 个开发者,2 个老手,1 个新人,2 人中等水平。我们仍在努力坚持每个人都要审核每个人 PR 的模式,但至少 1 个老手 + 1 个其他开发者的模式,将来可能更可行一点。
管理者审批为什么不算数?你也许认为“我是经理,也是个了不起的程序员,我的审核完全算数”,经理们的 review 确实有帮助,有大把管理者也是优秀的程序员,当他们有时间 code review 时,他们往往会有很好的建议或想法来帮助其他开发者。
但你必须考虑管理者花在编码上的时间有多少?如果超过 50%,那么他们也可以算作开发人员,这种情况作为审核者也是可以的。但是在我们公司,经理只是偶尔做一些非常偶然的编码工作,因此,主力还是要靠开发人员自己来说话。我不希望看到经理和开发人员之间的无限审批周期,一个管理者不断地审批一个个开发人员的PR,即使团队里还有 3 个开发人员有大把时间可以来做这个工作。
规则2
每个 PR 必须有一个好的描述,通过阅读描述,审核者能够理解代码的目的是什么。即使有一个 Jira ticket 或需求页面,也必须满足此要求。
没有描述的 PR —— 在我的团队中,这种情况永远不会通过。最好的情况是:他们没有写,是因为有另外一个 Jira ticket 页面有很好的描述。最坏的情况是什么也没有。
遇到没有描述的 PR,审核员有 2 个工作:
通过阅读 PR 中的代码修改,了解代码的作用。
审阅者试图夜观天象来判断代码是否做了它应该做的事情?
如果没有清楚地说明代码要做什么,就不可能审查它的正确性。你根本无法知道什么是正确的,你的操作是基于你认为正确的假设。
规则3
PR 必须有足够的单元测试和集成测试覆盖。
理想情况下,审阅者可以很容易地看到测试用例列表。
如果测试覆盖率已经存在,或者由于某种原因测试覆盖率不完整(工作被拆分到另一个依赖的 ticket),PR 描述中应该提到这一点。
给出好的 PR 评审意见很难、也很费时间。这是因为一旦你明白了代码应该做什么,你就必须停止审查并去查看你想要的测试覆盖率。你要合适的测试用例列表,并决定是哪种类型的测试:单元测试/集成测试/端到端。
你还必须考虑是否有可能对生产环境产生影响,以及是否需要在部署前对真实数据进行测试。
现在你知道了这些,你再去回顾一下 PR,看看他们是否涵盖了你所想的一切。理想情况下,这是一个混合体:你想到了他们没有想到的东西,他们也想到了你没有想到的东西,然后你们一起做得很好。这就是能够很容易看到 PR 中涵盖的测试用例列表的地方(相对于阅读 100 行或 1000 行的测试,必须弄清楚哪些用例是真正被测试的)。
注意,如果遵循了规则 2,你可以通过阅读描述就知道代码应该做什么。所以你可以在审查任何实际的代码之前,就可以完成审查测试覆盖率的步骤!
我个人喜欢把所有这些东西:相关的测试,真实数据测试,对线上客户端影响,生产环境配置更改都放在 PR 描述中。或者其他合适的地方,在一个地方统一查看。
规则4:
如果 PR 是一个 bug 修复,它必须包含一个测试,如果这个 bug 被还原,这个测试就会报错。
我以为这很明显,但就像大多数明显的事情一样,事情常常不是这样。
假设有一个打字错误的 bug,你用 != 代替了 ==,当时是晚上10点,你不应该去写代码,但你觉得那天晚上你想多做一些事情。于是你打开了一个 BUG 修复 PR,把 != 改成了 ==,有一个好心人也在工作,批准了它,然后把它送到了主分支。
三天后,你的好友 Duke 的 PR 也终于准备好了,他以为这是一个小功能,没有把它拆成小的 ticket,但事实并非如此,现在他有一个 2000 行的PR,他恳求团队进行代码审查。
Duke 执行了代码合并,代码冲突的地方你把 == 换成了 !=,Duke 有些疲惫,他没注意到这些细节,因此 != 又回来了。
对,因为没有测试来捕捉它,bug 就又回来了。
这是个虚构的例子,但如果你长期观察,这种情况经常发生。把团队的 bug 铺开 3 年,你会看到那些没有被测试捕获的 bug 又出现了。
总结
对,就这些! 实际只有 3 个规则,第 4 个更像是常识性的东西。
这些规则让我们的团队产生了一些质量相当不错的代码。对此,我很高兴,但也绝对不是我个人的功劳。
同样的,你也许认为这些规则对你也有帮助,但也许不太认同这些规则。不管怎样,如果这些规则引发了你的思考或想法,我很高兴。
原文:
https://medium.com/inside-league/how-one-code-review-rule-turned-my-team-into-a-dream-team-fdb172799d11
长按二维码 关注「高可用架构」公众号